Conversation
iSiRaH
commented
Feb 13, 2026
- Added locations tracking
- Store locations in database
- Enhance refresh token logic
- Added to fetch duty locations from backend
…rieval for field officers
There was a problem hiding this comment.
Pull request overview
Adds backend support for officer location tracking (persisting location points + retrieval APIs), introduces a duty-location lookup endpoint, and updates refresh-token handling to support validation + rotation.
Changes:
- Added
LocationPointpersistence model, DTO, repository, service, and controller endpoints for bulk upload + history/last-location retrieval. - Enhanced refresh token flow with
findValidToken()+ token rotation during/api/auth/refresh. - Added duty schedule location lookup (
/api/duty-schedules/locations) backed by a distinct-location query with defaults.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/crimeLink/analyzer/service/impl/LocationServiceImpl.java | Implements bulk save and history/last-location retrieval via repository |
| src/main/java/com/crimeLink/analyzer/service/LocationService.java | Introduces location service interface |
| src/main/java/com/crimeLink/analyzer/controller/LocationController.java | Adds endpoints for uploading and retrieving location data |
| src/main/java/com/crimeLink/analyzer/entity/LocationPoint.java | New DB entity for storing officer location points (incl. JSON meta) |
| src/main/java/com/crimeLink/analyzer/dto/LocationPointDTO.java | DTO record for mobile/client location payloads |
| src/main/java/com/crimeLink/analyzer/repository/LocationPointRepository.java | Repository methods for history and last-location queries |
| src/main/java/com/crimeLink/analyzer/service/RefreshTokenService.java | Adds validation + rotation support for refresh tokens |
| src/main/java/com/crimeLink/analyzer/repository/RefreshTokenRepository.java | Adds fetch-join query to load refresh token with user |
| src/main/java/com/crimeLink/analyzer/controller/AuthController.java | Updates refresh endpoint to rotate tokens and improve validation |
| src/main/java/com/crimeLink/analyzer/service/DutyScheduleService.java | Adds DB-backed duty location list with defaults |
| src/main/java/com/crimeLink/analyzer/repository/DutyScheduleRepository.java | Adds distinct location query |
| src/main/java/com/crimeLink/analyzer/controller/DutyScheduleController.java | Exposes duty locations endpoint |
| src/main/java/com/crimeLink/analyzer/config/SecurityConfig.java | Updates request matchers/authorization rules and CORS wiring |
| src/main/java/com/crimeLink/analyzer/config/JwtAuthenticationFilter.java | Adjusts public-path bypass list and adds verbose debug logging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (user == null) { | ||
| throw new RuntimeException("Unauthorized"); | ||
| } | ||
|
|
||
| if (!"FieldOfficer".equalsIgnoreCase(user.getRole())) { | ||
| throw new RuntimeException("Only field officers can upload locations"); | ||
| } | ||
|
|
||
| String officerBadgeNo = user.getBadgeNo(); | ||
| if (officerBadgeNo == null || officerBadgeNo.isBlank()) { | ||
| throw new RuntimeException("Badge number missing"); | ||
| } |
There was a problem hiding this comment.
This controller throws generic RuntimeException for auth/authorization/validation failures (e.g., "Unauthorized", "Only field officers..."). Without a @ControllerAdvice mapping, these will become 500 responses. Use proper HTTP statuses (e.g., ResponseStatusException with 401/403/400, or @PreAuthorize + validation) so clients get correct error codes.
| import com.crimeLink.analyzer.service.impl.LocationServiceImpl; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api") | ||
| @RequiredArgsConstructor | ||
| public class LocationController { | ||
| private final LocationServiceImpl service; | ||
|
|
There was a problem hiding this comment.
LocationController injects the concrete LocationServiceImpl instead of the LocationService interface. Prefer depending on the interface to keep the controller decoupled and make testing/mocking easier.
| @@ -52,9 +53,12 @@ protected void doFilterInternal( | |||
| } | |||
|
|
|||
| final String authHeader = request.getHeader("Authorization"); | |||
| System.out.println("🔍 Auth Header: " | |||
| + (authHeader != null ? authHeader.substring(0, Math.min(20, authHeader.length())) + "..." : "NULL")); | |||
|
|
|||
| // ✅ No token -> continue (SecurityConfig will decide permit/deny) | |||
| if (authHeader == null || !authHeader.startsWith("Bearer ")) { | |||
| System.out.println("❌ No Bearer token found"); | |||
There was a problem hiding this comment.
JwtAuthenticationFilter uses multiple System.out.println statements, including logging the Authorization header prefix. Avoid printing tokens/user data to stdout; use a logger with configurable levels (debug) and do not log any part of credentials/tokens in production logs.
|
|
||
| // 🔍 DEBUG: Log authentication success | ||
| System.out.println("✅ JWT Auth Success: " + userEmail); | ||
| System.out.println(" Authorities: " + userDetails.getAuthorities()); | ||
| System.out.println(" Accessing: " + path); |
There was a problem hiding this comment.
JwtAuthenticationFilter logs authentication success details (userEmail, authorities, path) to stdout. This is noisy and can leak security-relevant information. Switch to structured logging at debug level (or remove) and avoid logging authority sets for every request in normal operation.
| public void saveBulk(String officerBadgeNo, List<LocationPointDTO> points); | ||
|
|
||
| public List<LocationPoint> getHistory(String officerBadgeNo, Instant from, Instant to); | ||
|
|
||
| public LocationPoint getLastLocation(String officerBadgeNo); |
There was a problem hiding this comment.
Interface methods are implicitly public; the explicit "public" modifiers here are redundant noise. Consider removing them for consistency with typical Java interface style.
| public void saveBulk(String officerBadgeNo, List<LocationPointDTO> points); | |
| public List<LocationPoint> getHistory(String officerBadgeNo, Instant from, Instant to); | |
| public LocationPoint getLastLocation(String officerBadgeNo); | |
| void saveBulk(String officerBadgeNo, List<LocationPointDTO> points); | |
| List<LocationPoint> getHistory(String officerBadgeNo, Instant from, Instant to); | |
| LocationPoint getLastLocation(String officerBadgeNo); |
| // Allow duty schedule operations for OIC | ||
| .requestMatchers("/api/duty-schedules/**").hasRole("OIC") | ||
| // Public endpoints | ||
| .requestMatchers("/api/vehicle**").permitAll() |
There was a problem hiding this comment.
SecurityConfig permits "/api/vehicle**", but the VehicleController is mapped to "/api/vehicles". As written, vehicle endpoints likely won’t be publicly accessible as intended (and may unexpectedly require authentication). Update the matcher to the correct path (e.g., "/api/vehicles/**").
| .requestMatchers("/api/vehicle**").permitAll() | |
| .requestMatchers("/api/vehicles/**").permitAll() |
| .requestMatchers("/api/mobile/auth/**").permitAll() | ||
| .requestMatchers("/api/duties/**").permitAll() | ||
| .requestMatchers("/api/crime-reports/map").permitAll() | ||
|
|
||
| // Field Officer routes | ||
| .requestMatchers("/api/officers/me/**").hasRole("FieldOfficer") | ||
| .requestMatchers("/api/mobile/**").hasRole("FieldOfficer") |
There was a problem hiding this comment.
"/api/duties/**" is configured as permitAll. These endpoints expose duty assignments by officerId and date, which is sensitive operational data; permitting unauthenticated access is a security risk. Consider requiring authentication (e.g., hasRole("FieldOfficer")/hasAnyRole(...)) and enforcing that a field officer can only query their own duties.
| System.out.println("📍 LocationController.history() called"); | ||
| System.out.println(" Badge: " + officerBadgeNo); | ||
| System.out.println(" From: " + from + ", To: " + to); | ||
| System.out.println(" User: " + (user != null ? user.getEmail() : "NULL")); | ||
| System.out.println(" Role: " + (user != null ? user.getRole() : "NULL")); | ||
| System.out.println(" Authorities: " + (user != null ? user.getAuthorities() : "NULL")); | ||
| return service.getHistory(officerBadgeNo, from, to); |
There was a problem hiding this comment.
This controller logs request details (including user info, badge numbers, and time ranges) via System.out.println. Please replace with structured logging (logger) at an appropriate level and remove the verbose debug output before merge to avoid leaking sensitive data and spamming logs.
| public void uploadMyLocations(@AuthenticationPrincipal User user, @RequestBody List<LocationPointDTO> points) { | ||
| System.out.println("Received locations: " + points.size()); // REMOVE: for testing | ||
| if (user == null) { | ||
| throw new RuntimeException("Unauthorized"); | ||
| } |
There was a problem hiding this comment.
uploadMyLocations() calls points.size() before validating the request body. If the client sends a null body, this will throw a NullPointerException and return 500. Add a null/empty check (and return 400) before accessing points.